feat: support for the prompt parameter in the oidc flow#948
Merged
Conversation
dosubot
Bot
added
the
size:M
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesOIDC prompt support, auth_time claim, and oidc_prompt frontend parameter
Sequence Diagram(s)sequenceDiagram
participant Client
participant authorize as /authorize handler
participant AuthorizePage
participant LoginPage
participant authorizeComplete as /authorize-complete
Client->>authorize: GET with prompt parameter
alt prompt=none and unauthenticated
authorize-->>Client: OIDC error login_required
else prompt=none and authenticated
authorize->>AuthorizePage: redirect with oidc_prompt=none&oidc_ticket=...
AuthorizePage->>AuthorizePage: shouldAutoAuthorize=true
AuthorizePage->>authorizeComplete: useEffect invokes authorization mutation
authorizeComplete->>Client: return token with auth_time claim
else prompt=login
authorize->>LoginPage: redirect with oidc_prompt=login
LoginPage->>LoginPage: suppress post-auth redirect
LoginPage->>authorizeComplete: proceed after login with oidc_prompt cleared
authorizeComplete->>Client: return token with auth_time claim
else normal flow
authorize->>LoginPage: redirect to /login with oidc_prompt cleared
LoginPage->>authorizeComplete: proceed with oidc_prompt absent
authorizeComplete->>Client: return token with auth_time claim
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Rycochet
reviewed
Jun 19, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/service/oidc_service.go (1)
618-628:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPersist
auth_timefor refresh-issued ID tokens.
GenerateAccessTokencreates the OIDC session without savingcodeEntry.AuthTime, soRefreshAccessTokenhas to callgenerateIDToken(..., 0)and refreshed ID tokens omit the claim. StoreAuthTimewith the OIDC session and reuse it during refresh so clients don’t lose theauth_timecontract after the initial code exchange.Also applies to: 666-668
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/service/oidc_service.go` around lines 618 - 628, The CreateOIDCSession call is not persisting the AuthTime from codeEntry, causing refreshed ID tokens to lose the auth_time claim. Add the AuthTime field to the CreateOIDCSessionParams struct and pass codeEntry.AuthTime when creating the OIDC session. Then update RefreshAccessToken to retrieve the stored AuthTime from the OIDC session and pass it to generateIDToken instead of passing 0, ensuring the auth_time claim is preserved across token refreshes.internal/controller/oidc_controller.go (1)
179-219:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftKeep authenticated
prompt=nonerequests non-interactive.After the unauthenticated check, an authenticated
prompt=nonerequest still creates a frontend ticket and redirects to/oidc/authorize, which requires user interaction. Silent OIDC checks should either complete server-side and redirect with a code, or return an OIDC error such asinteraction_required/consent_required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/controller/oidc_controller.go` around lines 179 - 219, After the initial check for unauthenticated users with `prompt=none`, add additional logic to handle authenticated users who also have `prompt=none` set. When an authenticated user requests authorization with `prompt=none`, the request should not proceed to create an authorize ticket and redirect to the interactive `/oidc/authorize` screen. Instead, handle it non-interactively by either completing the authorization server-side and redirecting with an authorization code, or returning an OIDC error such as `interaction_required` or `consent_required` via the controller.authorizeError method. Add this check immediately after the existing `prompt=none` and unauthenticated validation and before the ticket creation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/controller/oidc_controller.go`:
- Around line 179-201: The current code uses exact string comparisons for
req.Prompt ("none" and "login"), but the OIDC prompt parameter is
space-delimited and can contain multiple values. Parse req.Prompt using
strings.Fields to extract individual tokens, then validate that if "none" is
present it cannot be combined with other values, and branch on token membership
instead of exact string equality. Update the condition checking req.Prompt ==
"none" to check if "none" exists in the parsed tokens, update the condition
checking req.Prompt == "login" to check if "login" exists in the parsed tokens,
and add validation to reject invalid combinations where "none" appears with
other values.
---
Outside diff comments:
In `@internal/controller/oidc_controller.go`:
- Around line 179-219: After the initial check for unauthenticated users with
`prompt=none`, add additional logic to handle authenticated users who also have
`prompt=none` set. When an authenticated user requests authorization with
`prompt=none`, the request should not proceed to create an authorize ticket and
redirect to the interactive `/oidc/authorize` screen. Instead, handle it
non-interactively by either completing the authorization server-side and
redirecting with an authorization code, or returning an OIDC error such as
`interaction_required` or `consent_required` via the controller.authorizeError
method. Add this check immediately after the existing `prompt=none` and
unauthenticated validation and before the ticket creation logic.
In `@internal/service/oidc_service.go`:
- Around line 618-628: The CreateOIDCSession call is not persisting the AuthTime
from codeEntry, causing refreshed ID tokens to lose the auth_time claim. Add the
AuthTime field to the CreateOIDCSessionParams struct and pass codeEntry.AuthTime
when creating the OIDC session. Then update RefreshAccessToken to retrieve the
stored AuthTime from the OIDC session and pass it to generateIDToken instead of
passing 0, ensuring the auth_time claim is preserved across token refreshes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 61178aab-f39a-4d47-813d-49d5d0f5193b
📒 Files selected for processing (6)
frontend/src/lib/hooks/screen-params.tsfrontend/src/pages/authorize-page.tsxfrontend/src/pages/login-page.tsxinternal/controller/oidc_controller.gointernal/model/context.gointernal/service/oidc_service.go
dosubot
Bot
added
size:L
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/service/oidc_service.go`:
- Around line 949-965: The GetPrompt function does not validate the OIDC Core
spec requirement that prompt=none cannot be combined with other prompt values.
Add validation logic within the GetPrompt function to detect when the input
contains "none" alongside other prompts and reject this invalid combination.
After splitting the prompt string, check if both "none" and other supported
prompts are present in the list, and if so, return an error indicator or
empty/invalid state rather than proceeding to return the first supported prompt.
This ensures requests like "prompt=none login" are properly rejected as
invalid_request instead of being accepted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 21f44d18-3908-4622-a031-980dc2766de5
📒 Files selected for processing (5)
frontend/src/lib/hooks/screen-params.tsfrontend/src/pages/authorize-page.tsxfrontend/src/pages/login-page.tsxinternal/controller/oidc_controller.gointernal/service/oidc_service.go
Rycochet
previously approved these changes
Jun 19, 2026
scottmckendry
previously approved these changes
Jun 19, 2026
steveiliop56
dismissed stale reviews from scottmckendry and Rycochet
via
2f46ff7
scottmckendry
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
promptparameter (none/login), including URL handling and backend enforcement.auth_time.Improvements
prompt=nonenow returns a standardized “login required” error when silent sign-in isn’t possible.prompt=nonewith other prompt values are rejected as invalid.